-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Deprecate ExprSchema
functions
#15847
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In deprecation note, state the fully qualified path to the replacement method, i.e. ExprSchemable::to_field
.
Marking as draft as I think this PR is no longer waiting on feedback and I am trying to clear review queue. Please mark it as ready for review when it is ready for another look |
Co-authored-by: Oleks V <[email protected]>
Co-authored-by: Oleks V <[email protected]>
Co-authored-by: Oleks V <[email protected]>
@@ -961,16 +961,19 @@ impl Display for DFSchema { | |||
/// widely used in the DataFusion codebase. | |||
pub trait ExprSchema: std::fmt::Debug { | |||
/// Is this column reference nullable? | |||
#[deprecated(since = "48.0.0", note = "Use ExprSchemable::nullable")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't the note point the user to use field_from_column
? This looks self referential.
fn nullable(&self, col: &Column) -> Result<bool> { | ||
Ok(self.field_from_column(col)?.is_nullable()) | ||
} | ||
|
||
/// What is the datatype of this column? | ||
#[deprecated(since = "48.0.0", note = "Use ExprSchemable::data_type")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment about field_from_column
fn data_type(&self, col: &Column) -> Result<&DataType> { | ||
Ok(self.field_from_column(col)?.data_type()) | ||
} | ||
|
||
/// Returns the column's optional metadata. | ||
#[deprecated(since = "48.0.0", note = "Use ExprSchemable::metadata")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment about field_from_column
ExprSchemable
functions ExprSchema
functions
I also think the comment is pointing from |
Which issue does this PR close?
Rationale for this change
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?